Skip to content

Better patch for #37 (missing sidebar on direct comic pages)#39

Merged
ilyvion merged 4 commits intoQuestionable-Content-Extensions:developfrom
trlkly:patch-3
Dec 8, 2019
Merged

Better patch for #37 (missing sidebar on direct comic pages)#39
ilyvion merged 4 commits intoQuestionable-Content-Extensions:developfrom
trlkly:patch-3

Conversation

@trlkly
Copy link
Copy Markdown
Contributor

@trlkly trlkly commented Dec 8, 2019

This patch is more idiomatic than #38, placing the change in the proper file and location (with comment), and using jQuery rather than raw JavaScript. It tests the DOM to see if the required column element (class ".small-2") is present and creates it if not, rather than testing the URL.

It also includes a version update in both the package and package-lock files, as well as an entry in the change-log, making for a more complete fix.

I realize you're busy, so I wanted to make things as simple as possible for you to help the fix get out quickly. Cheers!

More idiomatic fix for Issue Questionable-Content-Extensions#37. It uses jQuery within the proper file, just before adding the sidebar base. It also directly tests the DOM to see if the "small-2" column is missing, rather than just guessing using the URL.
No new features, just a bug fix , so only upped minor version number.
Minor version upped because bugfix--no new features
@ilyvion
Copy link
Copy Markdown
Contributor

ilyvion commented Dec 8, 2019

This might be just a Chrome thing (IIRC, you're a Firefox user), but this gap here isn't great...
image
It's caused by those (to be honest, broken, even without the extension) Privacy policy + dord links.

@ilyvion
Copy link
Copy Markdown
Contributor

ilyvion commented Dec 8, 2019

I tried finding a quick solution, but none came to mind...

@ilyvion
Copy link
Copy Markdown
Contributor

ilyvion commented Dec 8, 2019

I'll just approve it and publish it anyway, it's better than having nothing at all, for sure.

@ilyvion ilyvion merged commit d8fe68f into Questionable-Content-Extensions:develop Dec 8, 2019
@trlkly
Copy link
Copy Markdown
Contributor Author

trlkly commented Dec 8, 2019

Oh, I forgot about that. Someone in one of the subreddits already created a Userscript that fixed that, and I've been using that this whole time, with some of my own tweaks. The trick it uses is to move those links below the sidebar. (Though, if you wanted, you could move them anywhere.)

Here's the code from that extension:

// ==UserScript==
// @name         Questionable Content Image Resizer
// @namespace    QCIR
// @version      1.0
// @description  hotfix to the weird resizing of questionable content's images
// @author       Thierno Rignoux
// @match        *.questionablecontent.net/view.php*
// @match        *.questionablecontent.net/
// @match        *.questionablecontent.net/about.php
// @grant        none
// ==/UserScript==

(function() {
	'use strict';
	//move links to bottom of column
	var container = document.querySelector('#container .row .small-2');
	container.appendChild(document.querySelector('[href*="privacy.php"]') || document.createTextNode(''));
	container.appendChild(document.createElement('br'));
	container.appendChild(document.querySelector('[href*="dord.png"]') || document.createTextNode(''));
	container.appendChild(document.createElement('br'));
	container.appendChild(document.querySelector('[href*="urls.html"]') || document.createTextNode(''));
	//remove any extra text left behind
	var div = document.querySelector('#container > div');
	for (let i = 0; i < div.childNodes.length; i++) {
	  if (div.childNodes[i].nodeType === Node.TEXT_NODE) {
		div.childNodes[i].textContent = '';
	  }
	}
	//content.innerHTML = content.innerHTML.replace(/•/g,''); //dirty way to remove bullets. No longer needed.
})();

@trlkly
Copy link
Copy Markdown
Contributor Author

trlkly commented Dec 8, 2019

And here's the same code in jQuery:

let container = $('#container .row .small-2');
container.append($('[href*="privacy.php"]'));
container.append('<br>');
container.append($('[href*="dord.png"]'));
container.append('<br>');
container.append($('[href*="urls.html"]'));

Though, personally, I think moving it to where it is on the front page would make more sense. I'll look into that.

@trlkly
Copy link
Copy Markdown
Contributor Author

trlkly commented Dec 11, 2019

I also note that, if you don't have adblock enabled, the text does actually appear at the bottom of the page, so moving it down there is entirely consistent with his intention.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants